-
Notifications
You must be signed in to change notification settings - Fork 578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ Add validations to AWSCluster #1677
✨ Add validations to AWSCluster #1677
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify failed because awscluster_webhook_test.go is missing the boilerplate header.
Otherwise, lgtm
- Validates Region, ControlPlaneLoadBalancer, ControlPlaneEndpoint, and Bastion - Bastion can be added but not removed
d76974a
to
90d606b
Compare
/lgtm |
Removed the bastion validation |
/test pull-cluster-api-provider-aws-e2e |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: benmoss, detiber The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Adds validations to AWSCluster. I accidentally stumbled upon the fact that
region
was mutable, and the controller gets into a bad state if it's changed.I realized upon implementing this that it's rather difficult to enforce validations on
networkSpec
since it is repeatedly updated by the CAPA controller during reconciliation. I decided to punt on that since it was starting to turn into a much more complicated problem.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #1673